Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add check name in results #376

Closed
wants to merge 1 commit into from

Conversation

ernilambar
Copy link
Member

@ernilambar ernilambar commented Jan 8, 2024

In this PR:

  • Every Check is assumed to have a name. So get_name() has been added to interface Check.
  • check has been added to final class Check_Result.
  • includes/Traits/Amend_Check_Result.php - Methods are updated to accomodate newly added check field.
  • Late_Escaping_Check.php: get_name() method is implemented. Eg:
public function get_name() {
   return 'late_escaping';
}

Output:
Screenshot 2024-01-08 at 11 50 03 AM

@ernilambar ernilambar marked this pull request as draft January 8, 2024 06:03
@ernilambar
Copy link
Member Author

Requesting for feedback whether this approach is good or not.

cc: @swissspidy @felixarntz @mukeshpanchal27

@felixarntz
Copy link
Member

@ernilambar I like the idea of optionally supporting the inclusion of the check identifier. That said, I think we'll need to carefully think about how to implement that change. Preferably, this should be handled in a central place, without having to define the check identifiers as both array keys and in each class.

@ernilambar
Copy link
Member Author

@felixarntz May be this approach is better. Please review it. #380

@ernilambar ernilambar closed this Jan 11, 2024
@ernilambar ernilambar deleted the 329-add-check-name branch January 12, 2024 04:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants